-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix extending protected nested java classes #21857
Conversation
compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM
PR 21362 added an accessibility fix to Erasure, but that revealed a mistake in determining the accessibility of inner java classes, which I'm now fixing.
8b8d3bf
to
17dadf7
Compare
@@ -401,7 +401,8 @@ class ClassfileParser( | |||
classRoot.setFlag(sflags) | |||
moduleRoot.setFlag(Flags.JavaDefined | Flags.ModuleClassCreationFlags) | |||
|
|||
val privateWithin = getPrivateWithin(jflags) | |||
val jflags1 = innerClasses.get(currentClassName.toString).fold(jflags: Int)(_.jflags) | |||
val privateWithin = getPrivateWithin(jflags1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scala 2 sets the privateWithin
for inner classes when creating the symbol in enterOwnInnerClasses
Maybe Scala 3 should also do it there?
282f8d3
to
350215d
Compare
new ClassfileLoader(file), | ||
classTranslation.flags(jflags), | ||
getScope(jflags)) | ||
|
||
val privateWithin = getPrivateWithin(jflags) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if privateWithin.exists then
here as well for consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
Fixes #21631